Skip to content

[Main] Move to ListAll() calls for clusters to enable scalability testing #492

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
git-ival wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[Main] Move to ListAll() calls for clusters to enable scalability testing #492

git-ival wants to merge 1 commit into from
+9 −12

Conversation

@git-ival
Copy link
Contributor

@git-ival git-ival commented Dec 23, 2025

Issue: N/A

Problem

Currently, we use List() calls when making GETs for clusters. This is needed to enable higher scalability testing levels (> 1000 clusters).

Solution

Changed List() calls to ListAll() calls.

Testing

I have not tested on rancher/tests, but downstream in rancher/dartboard there were no regressions observed.

Regressions Considerations

Its possible there may be regressions if downstream users expect the original List() behavior.

@igomez06
Copy link
Contributor

igomez06 commented Jan 5, 2026

Due there being a possible regression, can we get this tested on the rancher/tests side to be sure?

@rancher-max
Copy link
Member

rancher-max commented Jan 5, 2026

For the purposes of review, putting the functions here https://github.com/git-ival/shepherd/blob/92397445854882ab3b438fd2e56299b674bfefac/clients/rancher/v1/client.go#L220-L260:

func (c *SteveClient) List(query url.Values) (*SteveCollection, error) {
	resp := &SteveCollection{}
	var jsonResp map[string]any
	url, err := c.apiClient.Ops.GetCollectionURL(c.steveType, "GET")
	if err != nil {
		return nil, err
	}
	url = url + "?" + query.Encode()
	err = c.apiClient.Ops.DoGet(url, nil, &jsonResp)
	if err != nil {
		return nil, err
	}

	err = ConvertToK8sType(jsonResp, resp)
	if err != nil {
		return nil, err
	}

	steveList := jsonResp["data"]
	for index, item := range steveList.([]any) {
		resp.Data[index].JSONResp = item.(map[string]any)
	}
	return resp, err
}

func (c *SteveClient) ListAll(params url.Values) (*SteveCollection, error) {
	resp, err := c.List(params)
	if err != nil {
		return resp, err
	}
	data := resp.Data
	for next, err := resp.Next(); next != nil && err == nil; next, err = next.Next() {
		data = append(data, next.Data...)
		resp = next
		resp.Data = data
	}
	if err != nil {
		return resp, err
	}
	return resp, err
}

@igomez06
Copy link
Contributor

igomez06 commented Jan 5, 2026

thanks @rancher-max so the difference really is between getting all clusters, or just a single page. Which seems like it won't be an issue in rancher/tests since they're not dealing with 100(0)s of clusters. If we can get someone from hostbusters to ok this. I'm good with approving it.

@git-ival
Copy link
Contributor Author

git-ival commented Jan 14, 2026

Hey @susesgartner @markusewalker 👋, I got pointed in your direction for some reviews 🫡

markusewalker
markusewalker approved these changes Jan 14, 2026
View reviewed changes
Copy link
Contributor

@markusewalker markusewalker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ran quick tests on rancher/tests and rancher/tfp-automation and it looked fine. If something ends up breaking, can address later 👍🏿

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@markusewalker markusewalker markusewalker approved these changes

@caliskanugur caliskanugur Awaiting requested review from caliskanugur caliskanugur is a code owner

@bmdepesa bmdepesa Awaiting requested review from bmdepesa bmdepesa is a code owner

@igomez06 igomez06 Awaiting requested review from igomez06 igomez06 is a code owner

@susesgartner susesgartner Awaiting requested review from susesgartner

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@git-ival @igomez06 @rancher-max @markusewalker